Tests: Precompile Tests Phase 3 (PLT-372) - #3985
Conversation
Extends the precompile suite to the non-wasm precompiles generated for v6.7 and to the methods added to the existing ones since phase 2, so the suite tracks the deployed interface rather than the phase 2 snapshot. New specs: auth (0x100D), authz (0x100E), evidence (0x100F), mint (0x1012), params (0x1013), slashing (0x1014), upgrade (0x1015). Existing specs grow to cover the scoped module authorizations and the query surface they shipped alongside: bank spendableBalances/totalSupply/ params/denomMetadata/denomsMetadata; staking grant/delegate/redelegate/ undelegate/revokeWithAuthorization plus the remaining delegation and validator queries; gov vote and proposal authorizations plus proposal/votes/deposits/tally; distribution withdrawValidatorCommission, the withdraw authorizations, and the validator/delegator queries. Framework: the new addresses join PRECOMPILE_ADDRESSES, cosmosUtils gains a cosmosRest LCD reader for the modules cosmjs does not wrap, and the bootstrap pool grows 48 -> 80 with the claim budget recorded. Wasm-gated flows (wasmd, pointer addCW*, solo), ibc, and the unregistered feegrant precompile stay out of scope. Co-Authored-By: Cursor <cursoragent@cursor.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #3985 +/- ##
==========================================
- Coverage 66.60% 60.24% -6.37%
==========================================
Files 2196 2054 -142
Lines 169180 176465 +7285
==========================================
- Hits 112689 106310 -6379
- Misses 56350 60428 +4078
- Partials 141 9727 +9586
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Phase 3 of the precompile suite is well grounded — every new address, ABI field name, LCD gateway path and Go revert string I spot-checked matches the deployed interface, and the claimPool budget note (29) is accurate. One new assertion in slashing.spec.ts compares two unpinned reads of per-block-mutating signing info and will fail intermittently; two smaller test-strength notes are non-blocking.
Findings: 1 blocking | 2 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- None at the file/PR level.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
slashing: signingInfos and signingInfo were two eth_calls at latest, and HandleValidatorSignature advances indexOffset for every validator in the last commit, so a block committing between them failed the equality with no defect behind it. Both reads now pin the same blockTag, which keeps the full-struct comparison rather than dropping the moving fields. bank: totalSupply sends no page limit, so the module default of 100 applies and every factory/... denom this suite mints sorts before usei. It now walks pages the way denomsMetadata already does instead of assuming usei is on page one. distribution: expectCoinArray degenerated to an "is an array" check on an empty array. It now requires non-empty by default, which the fixture guarantees for the delegation and validator reads, and communityPool opts out because sei-cosmos defaults community_tax to 0. gov: the dispatch section claimed gov has no view methods while a new case asserted proposal() answers under STATICCALL. The executor dispatches its queries before the readOnly check, so the label now says transaction methods, and the duplicate vote rejection that mined the same failing tx twice is gone. Co-Authored-By: Cursor <cursoragent@cursor.com>
|
Addressed all three of @seidroid's findings, plus the Bugbot note, in f996dac. Each claim was re-verified against the source rather than taken on the review's word. [blocker] [suggestion] [suggestion] One correction to the reasoning, since the conclusion is right but the mechanism isn't: the accruing delegation does not by itself guarantee [Bugbot, low]
|
The precompile suite read its Cosmos-side oracle from the LCD at localhost:1317. No container in the integration cluster publishes that port, so every cosmosRest call was refused and 15 tests failed. Replace cosmosRest with utils/moduleQueries.ts, which calls each module's protobuf Query service through abci_query on the Tendermint RPC port the suite already uses. That routes through the same handlers the node's gRPC server exposes, so the answers are identical and no cluster port changes. stakingHistoricalInfo now separates the module's not-found answer from a transport failure. The old .catch(() => undefined) read a refused connection as "this height is not retained" and then asserted the precompile must revert, which is what turned a connection failure into a "tx must fail" assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6c8f42e. Configure here.
| : null; | ||
| }, | ||
| { timeoutMs: 8_000, intervalMs: 200, label: 'validatorUnbondingDelegations' }, | ||
| ); |
There was a problem hiding this comment.
Unbonding queries outlive unbonding window
Medium Severity
After undelegate, this test sequentially waits up to 8s for unbondingDelegation and another 8s for validatorUnbondingDelegations. Devnet unbonding_time is 10s, so the second poll can run after the entry has matured and been removed, causing a flake rather than catching a precompile bug.
Reviewed by Cursor Bugbot for commit 6c8f42e. Configure here.
|
@seidroid review |
There was a problem hiding this comment.
Phase 3 of the precompile suite holds up under spot-checking: every new address, revert string, ABI field name and proto route I verified matches the deployed interface, and all three findings from the previous review are genuinely fixed. Only a small documentation gap remains.
Findings: 0 blocking | 1 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion]
README.md's directory tree (lines 106-113) enumerates every file underutils/but was not updated for the newutils/moduleQueries.ts, even though the PR does touch this README. That file is now the suite's primary Cosmos-side parity oracle (auth, authz, bank params, distribution, evidence, mint, slashing, staking historical info, upgrade) and is the first thing a new spec author needs to find. Add a line such as│ ├── moduleQueries.ts # abci_query parity oracle for modules cosmjs does not wrap. Relatedly, the PR description still describes this ascosmosUtils.cosmosRest, an LCD reader overSEI_REST— the implementation landed as rawabci_querythrough the shared Tendermint client instead, andSEI_RESTwas removed; worth correcting the description before merge so the commit message does not contradict the code.
Superseded: latest AI review found no blocking issues.
1 participant
Describe your changes and provide context
Phase 3 of the precompile test refactor (PLT-372), building on the phase 1 framework (#3777) and the phase 2 coverage (#3786).
Phase 2 covered the wasm-free precompiles as they existed then. Since that merged,
Generate v6.7 precompiles(#3961), the scoped module authorizations (#3893, #3846) and the module Msg rpc surface (#3825) landed. This phase brings the suite back in line with the deployed interface.New specs
auth.spec.tsaccount/accounts/params/nextAccountNumber, LCD parity incl. the nested base account module accounts wrap theirs inauthz.spec.tsgrants/granterGrants/granteeGrants; the non-empty fixture is astaking.grantStakingAuthorization, since authz itself cannot grantevidence.spec.tsevidence/allEvidenceagainst the module's own listmint.spec.tsparams/mintervs Sei's own/seichain/mint/v1beta1/...routesparams.spec.tsparams(subspace, key)vs the staking module, plus the unknown/empty subspace revertsslashing.spec.tsparams/signingInfo/signingInfos, grant + revoke of unjail authorization, and the unjail guardsupgrade.spec.tscurrentPlan/appliedPlan/upgradedConsensusState/moduleVersionsExisting specs extended
spendableBalances,totalSupply,params,denomMetadata(via a tokenfactory denom),denomsMetadata(pages until it reaches that denom, sonextKeyround-trips).grantStakingAuthorization→delegateWithAuthorization→ Cosmos-side delegation →revokeStakingAuthorization→ reverts;redelegate/undelegateWithAuthorization; and the remaining query surface (validators,validatorDelegations,unbondingDelegation,delegatorDelegations,redelegations,historicalInfo, …).grant/voteWithAuthorization/submitProposalWithAuthorization/revoke), plusproposal,proposals,getVote,votes,params,getDeposit,deposits,tallyResult.withdrawValidatorCommission, the withdraw authorizations, andparams,validatorOutstandingRewards,validatorCommission,validatorSlashes,delegationRewards,delegatorValidators,delegatorWithdrawAddress,communityPool.Framework
PRECOMPILE_ADDRESSES.cosmosUtils.cosmosRest— an LCD reader for the modules cosmjs does not wrap (mint, params, upgrade, evidence, slashing, authz, auth). Every path was taken from the module's decodedpb.gw.gogateway pattern rather than guessed, and every response field from its proto tag; the API server marshals withOrigName+EmitDefaults, so the reads are snake_case.claimPoolbudget (29) recorded next to it — a non-blocking note from the phase 2 review.Out of scope, unchanged: wasm-gated flows (wasmd, pointer
addCW*, solo), the ibc precompile, and feegrant (0x…1010), which is not registered and whose module was removed in #3958. oracle stays the phase 2 retirement assertion.Testing performed to validate your change
npx tsc --noEmitclean; no lint errors.historicalInfopinning an unreachable executor fallback instead of the querier'shistorical info for height %d not found;nextAccountNumberreadingnext_account_numberwhere the response field iscount; and mint's LCD path, which was three guesses deep with the real route last.historicalInfotry/catch asserting success in both branches,evidencehardcoding an empty chain in four places,authzindex-matching expirations across two independently ordered lists,bank.denomsMetadataasserting only "is an array" — and replaced receipt-only verification of the authorized reward withdrawal with theDelegationRewardsWithdrawncredit its non-authz sibling already pins.Not yet run against a live cluster. The remaining risk is concentrated in real chain responses, so this wants a
make docker-cluster-start+npm run precompile:cirun before merge; I will post the result on this PR.🤖 Generated with Cursor
Made with Cursor